Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

module: allow omitting context in synchronous next hooks #57056

Merged
merged 1 commit into from
Feb 18, 2025

Conversation

joyeecheung
Copy link
Member

This aligns the behavior of synchronous hooks with asynchronous hooks by allowing omission of the context parameter in the invocation of next hooks. The contexts are merged along the chain.

Fixes: #57030

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders

@nodejs-github-bot nodejs-github-bot added module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. labels Feb 14, 2025
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.
Copy link

codecov bot commented Feb 15, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.11%. Comparing base (79f96b6) to head (952c0f7).
Report is 32 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #57056      +/-   ##
==========================================
+ Coverage   89.10%   89.11%   +0.01%     
==========================================
  Files         665      665              
  Lines      193203   193216      +13     
  Branches    37220    37222       +2     
==========================================
+ Hits       172158   172190      +32     
+ Misses      13771    13758      -13     
+ Partials     7274     7268       -6     
Files with missing lines Coverage Δ
lib/internal/modules/customization_hooks.js 100.00% <100.00%> (ø)

... and 29 files with indirect coverage changes

@joyeecheung joyeecheung added the request-ci Add this label to start a Jenkins CI on a PR. label Feb 15, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Feb 15, 2025
@nodejs-github-bot

This comment was marked as outdated.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Feb 17, 2025

@joyeecheung joyeecheung added the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 18, 2025
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Feb 18, 2025
@nodejs-github-bot nodejs-github-bot merged commit ea2004a into nodejs:main Feb 18, 2025
57 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in ea2004a

joyeecheung added a commit to joyeecheung/node that referenced this pull request Feb 18, 2025
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.

PR-URL: nodejs#57056
Fixes: nodejs#57030
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
acidiney pushed a commit to acidiney/node that referenced this pull request Feb 23, 2025
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.

PR-URL: nodejs#57056
Fixes: nodejs#57030
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request Feb 24, 2025
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.

PR-URL: #57056
Fixes: #57030
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
targos pushed a commit that referenced this pull request Feb 25, 2025
This aligns the behavior of synchronous hooks with asynchronous
hooks by allowing omission of the context parameter in the
invocation of next hooks. The contexts are merged along the
chain.

PR-URL: #57056
Fixes: #57030
Reviewed-By: Jacob Smith <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TypeError: Cannot add property source, object is not extensible
6 participants